-
Notifications
You must be signed in to change notification settings - Fork 92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New allometric modes #1128
New allometric modes #1128
Conversation
…provements. 1. New allometric function for above-ground biomass (allom_amode = 4), which is similar to Chave et al. (2014), except that the exponent for wood density is independent of the one for "tree size" (i.e., DBH*DBH*Height). 2. New allometric function for leaf biomass and crown area (allom_lmode = 4), which uses tree size as the predictor. In the case of leaf biomass, the function also is a function of top-canopy SLA, using a similar formulation as the one for above-ground biomass. 3. Crown depth has now allometric modes too (variable allom_dmode). If allom_dmode=1, crown depth is a constant fraction of height; when allom_dmode=2, crown depth is defined as p1 * Height^p2. Former parameter crown_depth_frac was replaced with parameter allom_h2cd1 (p1), a new parameter allom_h2cd2 was added for the exponent. 4. All allometric mode variables were converted to integers when they are loaded. This avoids converting them to integers every time the allometric function is called. 5. Small edit to CheckParams. The current implementation of allom_hmode=2 is a bit unusual because the minus sign of the Weibull function is incorporated in parameter p2 (meaning that p2 must be negative). Also, instead of aborting the run as soon as an inconsistency in parameters is spotted, the subroutine now continues to check other parameters, and report all the errors before stopping the run. This avoids users submitting FATES multiple times to get one inconsistency at a time.
awesome set of updates @mpaiao looking forward to getting this integrated |
I was looking through an old issue related to fates Hydro and how it perceives and uses crown depth, see here: #674 I think the things we discuss in that thread are out of scope for this PR, but would be good to re-evaluate and move forward on them though. |
Just so people are aware, we don't call the crown depth allometry module in hydro: That crown depth routine is really only relevant for fire dynamics, as the radiative transfer doesn't use crown depth in its calculations. |
biogeochem/EDCanopyStructureMod.F90
Outdated
@@ -1616,12 +1625,10 @@ subroutine leaf_area_profile( currentSite ) | |||
! is obscured by snow. | |||
|
|||
layer_top_height = currentCohort%height - & | |||
( real(iv-1,r8)/currentCohort%NV * currentCohort%height * & | |||
prt_params%crown_depth_frac(currentCohort%pft) ) | |||
( real(iv-1,r8)/currentCohort%NV * crown_depth ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice update here @mpaiao
This will have a conflict with the new two-stream PR. I created a new subroutine that calculates the leaf area of crown layers, and it will need to leverage your fixes, see: https://github.com/NGEET/fates/pull/1141/files#diff-4fd8508e4a25680b45ecbfec6b6d80980a5b1e358af731203ee9d29266be46dbR2655
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for spotting this, @rgknox. Once the two-stream PR is integrated, I can update this part. I will leave this conversation open so we don't forget.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rgknox Should I go ahead and edit subroutine VegAreaLayer
now that two-stream has been merged?
@@ -305,7 +306,7 @@ subroutine h2d_allom(h,ipft,d,dddh) | |||
p3 => prt_params%allom_d2h3(ipft), & | |||
allom_hmode => prt_params%allom_hmode(ipft)) | |||
|
|||
select case(int(allom_hmode)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed, smarter to convert the switch on the read in, no need to convert over and over again in the dynamics loop
@@ -522,6 +534,12 @@ subroutine carea_allom(dbh,nplant,site_spread,ipft,crowndamage,c_area,inverse) | |||
call carea_2pwr(dbh_eff,site_spread,d2bl_p2,d2bl_ediff,d2ca_min,d2ca_max, & | |||
crowndamage, c_area, do_inverse) | |||
capped_allom = .true. | |||
case (4) | |||
dbh_eff = min(dbh,dbh_maxh) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noting that height capping is compatible with this scheme and implementation makes sense.
!--- Local variables | ||
real(r8) :: duse | ||
!---~--- | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
header and descriptions are nice and verbose, units, refs, great
return | ||
end subroutine size2dbh | ||
! ============================================================================ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You were very determined to find the inverse solution for dbh and h @mpaiao , not easy, kudos
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is totally borrowed from ED2 solvers, I just adapted it here. This allometric function is well behaved and should quickly converge with the Newton method, but I thought I should keep the full root finding approach in case we want to adapt for less well-behaved functions anywhere in FATES in the future.
write(fates_log(),*) '---~---' | ||
write(fates_log(),*) '' | ||
write(fates_log(),*) '' | ||
nerror = nerror + 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree.
if (nerror > 0) then | ||
write(fates_log(),*) 'One or more parameter errors found. Aborting.' | ||
call endrun(msg=errMsg(sourcefile, __LINE__)) | ||
end if |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
double checked to make sure that all checks removed endrun and added nerror, looks good
@mpaiao these changes look pretty clean to me. The root finding method is complicated, and I did not go through it in detail. But I did see that it was borrowed from ed2? If so, that is great, since its already been vetted. After discussion, I realized that the 2 is a derivative of an x^2. There is no precedent of explaining that in the other methods, so I have no further comments. Approvinng |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mpaiao - these are great changes/additions. I was wondering whether we want to put suggested parameter values for various allometry options, since default parameters will be wacky with some of the options. We already do this for the Chave d2h and dh2bagw but we could add it in for some others as well. Maybe out of scope for this PR, but might be helpful in future.
! degradation on water, energy, and carbon cycling of the Amazon | ||
! tropical forests. J. Geophys. Res.-Biogeosci. 125: | ||
! e2020JG005677. doi:10.1029/2020JG005677. | ||
! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new leaf biomass allometry looks great to me. One minor suggestion is to put suggested parameter values. It looks like the default d2bl1, d2bl2 and d2bl3 will give very high biomass with this equation so some suggested values for users wishing to use this option could be helpful.
write(fates_log(),*) '---~---' | ||
write(fates_log(),*) '' | ||
write(fates_log(),*) '' | ||
nerror = nerror + 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree.
…n the xml patch file and cdl
merge in cstarvation params and main, update xml patch file
Fix documentation typos and added suggested values for the new allometry (thanks JessicaNeedham)
… and alpha are incactive
protections metadata on eca phosphatase
added parameters and merged up
Bug fix. Adrianna Foster identified that trunks should be removed from the patch-level variable sum_fuel before being used to calculate rate of spread and fuel consumption.
Fixes exact restart bug for satellite phenology mode
I re-ran the fates regression tests on Location: |
Description:
allom_amode = 4
), which is similar to Chave et al. (2014), except that the exponent for wood density is independent of the one for "tree size" (i.e., DBHDBHHeight). Although this function could be simplified externally (by incorporating wood density into the scaler), this makes the dependency on wood density more transparent and may make parameter perturbation experiments a bit easier.allom_lmode = 4
), which uses tree size as the predictor. In the case of leaf biomass, the function also is a function of top-canopy SLA, using a similar formulation as the one for above-ground biomass.allom_dmode=1
, crown depth is a constant fraction of height (the former default); whenallom_dmode=2
, crown depth is defined as p1 * Height^p2, following Poorter et al. (2006). Former parametercrown_depth_frac
was replaced with parameterallom_h2cd1
, and a new parameterallom_h2cd2
was added for the exponent.Collaborators:
Expectation of Answer Changes:
Checklist
If this is your first time contributing, please read the CONTRIBUTING document.
All checklist items must be checked to enable merging this pull request:
Contributor
Integrator
Documentation
Test Results:
CTSM (or) E3SM (specify which) test hash-tag:
CTSM (or) E3SM (specify which) baseline hash-tag:
FATES baseline hash-tag:
Test Output: